Skip to content

Conversation

@soumeh01
Copy link
Collaborator

@soumeh01 soumeh01 commented May 26, 2025

@qltysh
Copy link

qltysh bot commented May 26, 2025

❌ 1 blocking issue (1 total)

Tool Category Rule Count
prettier Style Incorrect formatting, autoformat by running qlty fmt. 1

@qltysh one-click actions:

  • Auto-fix formatting (qlty fmt && git push)

Copy link
Collaborator Author

@soumeh01 soumeh01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a few observations:

  • We can't rely solely on the top-level mkdocs hashes, as pip requires hashes for all dependencies. Without including hashes for transitive dependencies, pip raises errors and refuses to install the incomplete requirements.
  • The PR should resolve the linked issue. However, the auto update of the pip dependencies can only be tested when Dependabot creates a new update for the pip dependency. Until then, the expected behavior is that the changes should not alter existing functionality.

repository: ${{ github.event.pull_request.head.repo.full_name }}
token: ${{ secrets.GITHUB_TOKEN }}

# - name: Set up Python
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still needed?

pull_request:
paths:
- '.github/workflows/mkdocs.yml'
- '.github/requirements.in'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be the .in file or the requirements.txt file with the hashes.

@@ -0,0 +1,50 @@
name: Recompile requirements.txt on Dependabot PRs
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have control over running the actions? I.e. to make sure that the mkdocs workflow only runs after the pip-update workflow? Apparently, there is an optional but strong dependency between the two.

name: Recompile requirements.txt on Dependabot PRs

on:
pull_request_target:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 so this workflow's automated commits only happen on a branch, never on main?

@@ -0,0 +1,215 @@
#
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is requirements.txt the standard name of such lock files for pip?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is the standard name for Python pip packages.

@jreineckearm
Copy link
Collaborator

@soumeh01 , please pause your work on this. I see a different PR #283 where mkdocs goes away. Let me check first what's the story there.

@soumeh01 soumeh01 added the do not merge Do not merge yet. Something else needs to happen first label May 26, 2025
@jreineckearm
Copy link
Collaborator

@soumeh01 , just to confirm that the idea of #283 is indeed to move documentation to a single MD file readable through extension registries like VS Code marketplace and in-tool MD readers showing during extension management. Anyway, thanks a lot for the investigation. It's good to understand what we can do in this area for PIP dependencies.

I would suggest to wait with closing the PR until the other PR gets merged. Let's also make sure to keep the approach somewhere as an example for future pinning of PIP dependencies. I got a feeling we'll need this again sooner than we think...

@soumeh01
Copy link
Collaborator Author

Closing this PR as the changes are redundant as mkdocs are no longer used.

@soumeh01 soumeh01 closed this May 28, 2025
@jreineckearm jreineckearm deleted the pinned-pin-deps branch June 18, 2025 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do not merge Do not merge yet. Something else needs to happen first

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants